[Lens] Support histogram mapping type for all numeric functions #90357
Conversation
wylieconlon
changed the title
wylieconlon
added
Feature:Lens
release_note:feature
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
| @@ -102,6 +102,9 @@ export const fieldMappings = { | |||
| bytes: { | |||
| type: 'long', | |||
| }, | |||
| bytes_histogram: { | |||
| type: 'histogram', | |||
| }, | |||
There was a problem hiding this comment.
For reviewers: You can test the histogram data in the sample logs, but I think this is probably not a good idea to merge. So I'll plan to remove these changes before merging.
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
There was a problem hiding this comment.
I would be fine with solving this separately, but as there's no default formatter on the index pattern for histogram fields, we fall back to Number.toString which is not the best choice:
Three options I see here:
- Set default number formatter for histogram fields on the index pattern (this would solve the issue for Visualize charts as well)
- Have a special logic for histogram fields in Lens
- Just ignore for now
Approving for the general feature, if we are going to merge without addressing the formatting, let's open a separate issue
| @@ -205,6 +209,81 @@ export async function getNumberHistogram( | |||
| }; | |||
| } | |||
|
|
|||
| export async function getNumberOnlyHistogram( | |||
There was a problem hiding this comment.
nit: It looks like this code is almost the same as getNumberHistogram (except for the top values part). Can we reduce the redundancy somehow in a nice way (maybe some conditional branches)? Not a blocker, just stumbled over it a little.
|
@flash1293 The number formatting for histograms is a little complicated because we need to apply different formatting for the raw value vs aggregated value. So in Discover, we use a string formatter, but in Lens we use a number formatter. My preference is to change the formatter at the aggConfigs level somehow. I was having a hard time with the |
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
|
@flash1293 I already opened an issue for app-services here: #91163 but I plan on spending a little time to define the solution more. |
…tic#90357) * [Lens] Support histogram mapping type * Fix field stats and allow max/min * Fix types * Revert to regular sample data * Simplify server code * Add test for edge case Co-authored-by: Kibana Machine <[email protected]>
…) (#91521) * [Lens] Support histogram mapping type * Fix field stats and allow max/min * Fix types * Revert to regular sample data * Simplify server code * Add test for edge case Co-authored-by: Kibana Machine <[email protected]> Co-authored-by: Kibana Machine <[email protected]>
This is supported in field stats too:
Closes #74581
Checklist